Consider tracking precursor origins on null principals
Categories
(Core :: Security: CAPS, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox92 | --- | fixed |
People
(Reporter: nika, Assigned: nika)
References
(Blocks 1 open bug)
Details
Attachments
(7 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
Null principals are, by definition, always opaque and unique, and don't subsume anything, so when we create them we never keep track of the "precursor origin" which caused the null principal to be created (e.g. if there was a sandboxed origin, the precursor origin would be the origin which was sandboxed to create the null principal, or when loading a data: URI the precursor origin would be the origin which would be inherited if data: URIs still inherited principals when loaded as documents).
For process selection stuff related to null principals, though, I think it would be quite useful to track this information on the null principal. It wouldn't impact the way that they compare at all, but would instead be an extra bit of information carried around with the principal, meaning that we can avoid needing a separate process for every null principal (as that would likely be prohibitively expensive), while also avoiding putting information which came from different sites but were both erased to a null principal in the same process.
A potential implementation for this would be to include the precursor origin in a null principal's origin as a query, like: moz-nullprincipal:{UUID-GOES-HERE}?https://example.com
. This shouldn't impact the existing semantics of origins, as the precursor origin would need to be provided at creation time, and we (as far as I know) never construct a null principal from its bare UUID, so the precursor origin will always either be present or not present.
The precursor origin would be recorded whenever a principal is created with methods like NullPrincipal::CreateWithInheritedAttributes
, and wouldn't include OriginAttributes
, as the null principal already tracks those separately.
I have marked bug 1714645 as blocking this bug, as using a standard URI format like nsSimpleURI
will make adding the extra query component much simpler.
Comment 1•4 years ago
•
|
||
As discussed, it think this bug would also solve the problem of whether a NullPrincipal originated from a secure-context as discussed in Bug 1660452.
Updated•4 years ago
|
Assignee | ||
Comment 2•4 years ago
|
||
I actually already have an implementation of this locally, and forgot to assign it to myself. I mostly haven't posted it yet because I've been focusing on the changes we need to do for fission in bug 1650089 which will build upon these changes.
Have you gotten very far with the work on this so far, or should I take this bug back & post my current changes?
Comment 3•4 years ago
|
||
Tracking for Fission M7a because this bug blocks Fission M7a bug 1650089.
Updated•4 years ago
|
Assignee | ||
Comment 4•4 years ago
|
||
This is currently temporarily stalled on some issues around null principal generation for sandboxed loads, as currently the principal used by a sandboxed load is generated based on the loading principal here: https://searchfox.org/mozilla-central/rev/9c9f2f85d00aef1943cb521ac95c72bae932ebc5/netwerk/base/LoadInfo.cpp#843-844. This was done in bug 1335526 to ensure that multiple calls to get the channel's result principal for a sandboxed load will return the same null principal, rather than generating a different null principal each time.
Unfortunately, we don't want to base the precursor principal off of the loading principal, we want to base it off of the non-sandboxed channel result principal. This would require us to be able to generate a new null principal for each call to GetChannelResultPrincipal
, so that we can include the non-sandboxed principal in the null principal's origin.
An "easy" workaround for this would be to include the null principal UUID to use for the sandboxed null principal on the LoadInfo such that we can generate the same null principal for each call, so long as GetChannelResultPrincipalIfNotSandboxed
returns the same origin. However, this would mean that we could end up with multiple null principals with the same UUID but different precursors, which, while it may not be an issue (they won't compare .Equals(...)
), seems unexpected and potentially wrong. :ckerschb and I are discussing various other options for doing this such as by using a random salt and the precursor in a hash to generate the UUID, but we're not sure what the final option here should be.
It may be possible for us to find a general solution which handles all null principals which can be generated from GetChannelResultPrincipal
(including those generated due to e.g. data:
URI loads), to make sure that we return consistent results from repeated calls with the same channel.
(In reply to Chris Peterson [:cpeterson] from comment #3)
Tracking for Fission M7a because this bug blocks Fission M7a bug 1650089.
Moving to M8 as bug 1650089 is actually a M8 bug
Assignee | ||
Comment 5•4 years ago
|
||
This patch only adds the machinery for tracking a precursor origin to the
principal, and does not actually track the precursor origin in any situations.
That is done in follow-up patches.
Assignee | ||
Comment 6•4 years ago
|
||
This method will be the primary way to track the precursor for a null
principal, and will automatically handle tracking precursors in some common
cases.
While sandboxed principals are created with CreateWithInheritedAttributes
,
they unfortunately currently use the wrong precursor principal, which will be
fixed in a later part.
Depends on D119688
Assignee | ||
Comment 7•4 years ago
|
||
If a URI has the URI_INHERITS_SECURITY_CONTEXT flag it will not be given
a content principal by CreateContentPrincipal. This patch changes the
algorithm for creating result principals for network requests such that
the null principal created in this situation has a precursor principal
tracked on it.
Depends on D119689
Assignee | ||
Comment 8•4 years ago
|
||
When a navigation redirects, the principalToInherit is reset back to a null
principal for security reasons. This helps prevent a redirect from loading
attacker controlled content with the wrong principal. This patch gives this new
principalToInherit a precursor origin based on the resource which is being
redirected.
One time when this may come up is when an extension redirects a http: request
to a data:
URI through the request API. Before this change, the load would
complete with a precursorless null principal, but after this change the
precursor refers to the redirected-from URL.
Depends on D119690
Assignee | ||
Comment 9•4 years ago
|
||
This provides a getter which can be used to interact with the precursor
attribute of the null principal.
Depends on D119691
Assignee | ||
Comment 10•4 years ago
|
||
This change stores a generated nsID directly on the LoadInfo, rather
than the full SandboxedLoadingPrincipal. This allows for the sandboxed
principal to be constructed from GetChannelResultPrincipal using the
unsandboxed result principal as a precursor, rather than the loading
principal.
The nsID is reset by HttpChannelBase whenever a non-internal redirect
occurs to reduce the chance of multiple null result principals during a
redirect with the same nsID, but different precursors.
Depends on D119692
Assignee | ||
Comment 11•4 years ago
|
||
These test various ways of loading documents which will end up with null
principals, and verify that they are loaded with the expected precursor URI.
Depends on D119693
Comment 12•4 years ago
|
||
Comment 13•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/95615f0cb5de
https://hg.mozilla.org/mozilla-central/rev/06ae8315c75d
https://hg.mozilla.org/mozilla-central/rev/a928d061642f
https://hg.mozilla.org/mozilla-central/rev/60f3609def05
https://hg.mozilla.org/mozilla-central/rev/116910d82bf1
https://hg.mozilla.org/mozilla-central/rev/70f7bc70e89f
https://hg.mozilla.org/mozilla-central/rev/99863d6b8f8a
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Description
•